-
-
Notifications
You must be signed in to change notification settings - Fork 894
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
PushInfo is documented to return a local reference, but a pushed commit need not be a reference #1624
Comments
Thanks for looking into this and offering a fix, it's much appreciated! I suggest to keep Feel free to experiment and find other, better solutions though, everything goes except breaking the existing API :D. Thanks for your understanding. |
Yeah, it's tricky because it kind of depends on what counts as breaking the existing API... and I've seen maintainers take all sorts of positions on this. Adding an extra local_hash field and setting local_ref to None seems reasonable, and something like that occurred to me even. The potential problem I see is that there might well be code out there that does something like push_infos = repo.push(...) which would previously "work", but would now crash. A potentially safer approach, if we're worried about this, could be to just reroll the API entirely, i.e. add a "Repository.push2()" method. We don't even need to reroll PushInfo in this case; we just need to make sure that the v1 .push() never returns a PushInfo with a local_ref of None. |
Thanks for the analysis!
Thanks again for your continued help with this! |
Hi!
It is a perhaps little known fact that one can use
git push
to push something that is not a reference, with syntax likegit push 1a2b3c4d:refs/heads/remotebranch
This is possible with GitPython, but subtly broken. The Repo.push() command technically succeeds, but returns a zero-length array and no errors.
This is because when building a PushInfo struct, GitPython expects the local commit that is pushed to be a reference, and this is not always true. When unable to convert the local commit into a reference, GitPython simply gives up and doesn't build that PushInfo.
Fixing this would be relatively trivial, and I'd love to submit the patch to do it (props, by the way, for writing very straightforward and readable code.) The trouble is that this specific detail is documented:
and any change we do here would imply breaking the API. How should we deal with this? I'd be glad to implement whatever solution is decided on.
The text was updated successfully, but these errors were encountered: