GitPython icon indicating copy to clipboard operation
GitPython copied to clipboard

Is gitdb an implementation detail?

Open tucked opened this issue 6 years ago • 5 comments

git.Repo(path).commit(bad_hash)

This raises gitdb.exc.BadName (which is not catchable with git.exc.GitError "Base class for all package exceptions").

Do I need to catch gitdb exceptions too? I don't have a dep on gitdb otherwise.

tucked avatar Oct 10 '19 01:10 tucked

Yes, gitdb is part of the requirements https://gitpython.readthedocs.io/en/stable/intro.html#requirements

stsewd avatar Oct 10 '19 01:10 stsewd

Those are GitPython's requirements, though, not my requirements... For example, gitdb2 depends on smmap, but GitPython doesn't have that in its requirements.txt.

tucked avatar Oct 10 '19 02:10 tucked

Thanks for posting! I believe this is indeed an issue, and it's worth fixing for better usability and less surprises. Historically, GitDB was meant as standalone low-level library anyone could use, but practically it's only used by GitPython. And if memory serves, at some point I did my best to reduce the dependency by using the git <command> backend instead. However, depending on the method, GitDB is still used as indicated here.

I believe everyone would benefit by removing or merging GitDB into GitPython for good, and PRs are very welcome.

Byron avatar Oct 10 '19 06:10 Byron

That sounds like a big task... In the meantime, maybe something like this could suffice:

try:
    gitdb.op()
except gitdb.exc.ODBError as exc:
    raise git.exc.GitDBError from exc

Actually though, I just realized that GitPython (not GitDB) is raising gitdb.exc.BadName: https://github.com/gitpython-developers/GitPython/blob/23b83cd6a10403b5fe478932980bdd656280844d/git/repo/fun.py#L147

So, maybe more like:

class BadName(git.exc.GitError, gitdb.exc.BadName):
    pass

raise BadName(name)

edit: FWIW, my workaround is to catch git.exc.BadName directly which works because GitPython imports gitdb exceptions: https://github.com/gitpython-developers/GitPython/blob/23b83cd6a10403b5fe478932980bdd656280844d/git/exc.py#L8

tucked avatar Oct 11 '19 16:10 tucked

@tucked Wow, fantastic write up! I would be so happy if this could be fixed for everyone with what seems like a small PR. (Still catching up on emails, maybe the PR is already there)

Byron avatar Oct 15 '19 11:10 Byron