Note in post_clear_cache docstring may be out of date

[EliahKagan]

The git.index.util.post_clear_cache decorator has this note in its docstring:

GitPython/git/index/util.py

Lines 72 to 74 in fe1934c

	:note:
	This decorator will not be required once all functions are implemented
	natively which in fact is possible, but probably not feasible performance wise.

This decorator is applied to the merge_tree, move, and remove methods of git.index.IndexFile (and nowhere else).

As such, that note in the decorator's docstring suggests an intended design direction for the IndexFile implementation in GitPython: that, if performance considerations permit, it should eventually make little to no use of git commands, unlike GitPython as a whole.

By now, my guess is that the intended direction is known, and my (somewhat less certain) guess is that this goal may have been abandoned, becuase:

• GitPython is considered to be in maintenance mode (per the README).
• In a review comment, if I understood correctly, you have expressed the hope that IndexFile is not heavily used, due to not having reached some of its original design goals.

However, there is also now a longer-term vision for GitPython, beyond maintaining the current feature set, articulated in Byron/gitoxide#1074 under "GitPython…".

If in the future the IndexFile implementation will be rewritten to use gitoxide's (future) Python bindings, then this docstring note is actually of greater relevance than before. But then it should still be updated to no longer characterize the aspiration to implement all of IndexFile without git subprocess calls as infeasible for performance reasons. And possibly even to mention gitoxide?

Because of that, I'm unsure if this note should be removed or updated (if, if updated, then how), or simply left as-is for the time being.

[Byron]

Thanks for reminding me!

I think it's fine to update the doc-string to be a better fit for how the decorator is used right now, and let the future happen independently.

[EliahKagan]

I think it's fine to update the doc-string to be a better fit for how the decorator is used right now, and let the future happen independently.

The most straightforward way to do that may be to remove the note, though it could also be changed to say something like:

This decorator is required because not all functions related to IndexFile are implemented natively.

(Or a less vague version of that may be feasible.)

Edit: I have just also realized that there may be ambiguity in how "natively"--present both in the original wording and in the possible new wording--may be read here, since it doesn't refer to anything related to native code, but instead means self-contained in GitPython and not using the git binary. I'm not sure expanding it would improve clarity, though.

[Byron]

Thinking about it, I also think it is probably best removed. When GitPython gets access to gitoxide, one has to see how it should change as well. My guess is that ultimately, for compatibility, GitPython has to stay exactly as it is those who can start fresh can just use gitoxide from Python. So it's fine to act like my remark about gitoxide doesn't exist 😅.

[EliahKagan]

Thinking about it, I also think it is probably best removed.

I will include the note's removal in a forthcoming pull request with some other docstring revisions.

My guess is that ultimately, for compatibility, GitPython has to stay exactly as it is

post_clear_cache is listed in git.index.util.__all__, so it's true it cannot be removed, and it decorates public methods of git.index.base.IndexFile, but it seems to me that which methods (if any) it decorates is an implementation detail that could potentially change in the future, even without a major version change, without breaking anything or leading to any unpleasant surprises. It may be that I have not looked deeply enough into this, though.

In particular, if the goal that code that uses GitPython performs git subprocess calls for exactly the same cases it currently does (except as necessary to fix bugs or for other situations with specific decisive benefits) then I can see that this must not change.

Even if so, however...

So it's fine to act like my remark about gitoxide doesn't exist 😅.

...I hope that by this you do not mean more broadly that you've abandoned the aspiration for GitPython articulated in Byron/gitoxide#1074.

Even if it is a requirement that GitPython (even in a future major version) never cause any currently working GitPython-using Python program to use gitoxide instead of a git subprocess, I think it should still be feasible to allow gitoxide to be used when code that uses GitPython is slightly modified, for example to use newly introduced subclasses of Git and Repo that use gitoxide for applicable operations (or, if increased reliance on inheritance is to be avoided, then by some other means).

I expect this would have two benefits: code that currently uses GitPython could be gradually migrated to use gitoxide, including evaluating if doing so would likely yield performance gains in specific applications; and both old and new code could be made to use gitoxide when available with immediate fallback to making git subprocess calls. Personally (since I am not maintaining any programs or libraries that use GitPython directly), this second benefit is what most speaks to me.

The dependency on gitoxide's Python bindings (and thus on gitoxide itself) could be made optional by expressing it as an extra, so that this feature would require both small code changes to opt in and either specifying a GitPython dependency as GitPython[gitoxide] or otherwise separately having the gitoxide bindings as a project dependency.

I emphasize that my knowledge of gitoxide (and possibly other relevant knowledge) is insufficient at this time for any of this to really constitute an informed opinion. But these are my hopes, relating to the GitPython vision expressed in Byron/gitoxide#1074.

[Byron]

n> Even if so, however...

So it's fine to act like my remark about gitoxide doesn't exist 😅.

...I hope that by this you do not mean more broadly that you've abandoned the aspiration for GitPython articulated in Byron/gitoxide#1074.

I dropped the ball there for a moment 😁.

First off, it's great to hear that a gitoxide infusion can be done as opt-in and possibly with new stand-in types for Repo or Git that then change behaviour. And maybe that is still the way to go assuming that people would actually use it. The reason to use it should be performance and correctness, I presume, even though correctness is a topic that can already be tackled as its mostly hidden in the types that GitPython uses (I am looking at you, encoding-errors).

New projects could just start out with what hopefully will be a the better API of the gitoxide python bindings, so bringing old projects to use a new backend without enforcing it seems like a tough problem to solve. It's also the question if the current API of GitPython is so great that it really should be preserved like that. Strangely, right now I can't see myself working on this at all, even with gitoxide python bindings available, just because I have no hope that the value proposition is strong enough (especially compared to the risk of things breaking).

What I can imagine working on though is to make gitoxide python bindings so nice that GitPython wouldn't stand a chance in comparison - in a way, GitPython can't change much at all as one of its major value propositions by now is that it is stable.

Maybe this analysis is wrong, and that's fine, as there is still a long way to go until gitoxide python bindings even can exist. And then I hope we talk again about what's possible :).

[EliahKagan]

What I can imagine working on though is to make gitoxide python bindings so nice that GitPython wouldn't stand a chance in comparison

Yes. That sounds like a very valuable thing regardless of what else, if anything, there is.

Another thought is that by this time it may be easier than today to implement automated code transformations that translate code between conceptually dissimilar APIs with minimal manual fixup. The easier and more reliable such a thing is, the less reason there may be to enhance GitPython to include gitoxide as a backend, and the more reason there may be to implement such a transformation tool, which might hopefully in the future be easier than today.