Review exceptions

[jdavid]

To have one exception for every GIT_EXXX all of them inheriting from GitError, use a consistent naming, e.g. GIT_EINVALIDSPEC would become GitInvalidSpecError or maybe just InvalidSpecError.

To provide backwards compatibility use multiple inheritance, e.g. InvalidSpecError would inherit from GitError and ValueError. Eventually the base class for backwards compatibility would be remove in some future version.

To settle on naming post here the full mapping of GIT_EXXX to exception, even if this can be implemented in steps.

This follows up on issues #531 and #828

[robinst]

Sounds good to me.

Eventually the base class for backwards compatibility would be remove in some future version.

There's some precedent in Python's standard library for exceptions with multiple base classes, see this post:

https://www.reddit.com/r/Python/comments/1k9ygb/multiple_inheritance_for_library_exceptions/

So maybe it's not necessarily a bad thing that needs to be removed in the future.

[jdavid]

yes you're right, it may be a good thing

[omniproc]

While I implemented a first version there are currently some reasons that stop us from fully switching to such a system that will probably need to be tackled one-by-one.

• [ ] Passthrough needs to be changed to GitPassthroughError to be consistent in the naming. The issue is that while I replaced all instances in PyGit2 with the new name, Passthrough might be used by users. I therefor let GitPassthroughError inherit from Passthrough and didn't delete Passthrough yet. So, Passthrough can be viewed as deprecated and some time in future should be removed. Also since I'm not sure what side effects Passthrough has on the flow logic (it is a special error, just like GIT_EUSER) I didn't yet add it to the automatic error generation, it still has to be explicitly raised by some Python code.
• [ ] GIT_EUSER is currently used to control the flow of some wrappers. It seems the current logic that uses GIT_EUSER return codes needs to be changed in a way that it's using the new GitUserError exception. This seems to be quite hard as I yet don't fully understand the flow logic there. Some help with this would be awesome.
• [ ] Because of that mentioned GIT_EUSER (and the related _stored_exception logic) issue and the fact that some exceptions are currently catched and never raise it's not clear if #996 is fixed with this implementation or if the GIT_EUSER thing first needs to be fixed/replaced. The functions of interest seem to be _fill_fetch_options, _fill_push_options, _fill_prune_callbacks, _fill_connect_callbacks and all callers of those as well as the @ffi.callback wrapper.
• [ ] The old exception handling used check_error(err, io=False) and set io=True if it wanted a ValueError to become a IOError. I'm not sure what was the reason for that but to be backwards compatible I kept it that way. Every line that previously made a call to check_error() with io=True was replaced by a special, temporary exception class GitIOError. You can find the relevant parts if you search for GitIOError.check_result(. Those parts need to be changed in a way that the actual GIT_E error is thrown and then GitIOError can be removed. Since I'm not sure what's the reason behind the explicit raise of IOError instead of other exceptions (e.g. ValueError) in first place this would need some clearification first.
• [ ] So far I only implemented the GIT_E* codes (and their matching classes) for the errors that are currently explicitly handled. Before all the remaining codes are implemented it would probably be a good idea to discuss what C calls currently occurre and what errors they could raise, and then add the new handling logic to them as needed. I marked the not yet implemented GIT_E* variants with a TODO in errors.py.