libgit2/pygit2

Repository.diff fails when treeish is "empty"

Inkane opened this issue · 8 comments

Repository.diff contains https://github.com/libgit2/pygit2/blob/master/pygit2/repository.py#L390
This causes a subsequent failure when a is pygit2.Commit and len(a.tree) is 0, because a treeish_to_tree(a) evaluates to False in this case. Consequently, a will be the Commit object afterwards.
An example for such a commit is eb02887bc34130647a52f8152c13531d97f5a058 from https://github.com/twbs/bootstrap.git . https://stackoverflow.com/questions/9765453/gits-semi-secret-empty-tree might shed some light on the empty tree.

So git is assuming that every other implementation is going to support the fake empty tree and it doesn't bother writing the actual tree it wants to use to disc? That is quite worrying.

This behaviour has nothing to do with git-core's special handling of the empty tree in some situations, or really with the Git system at all, it's purely an unfortunate decision in Python to consider that an object with a sequence interface is falsy just because the length of that aspect is zero.

I disagree with the solution. It is the code in Repository.diff which is wrong and should be spelled
differently. It should be explicit.

Regarding the if len == 0 then falsy debate, I have not yet stop to think about it, so I don't
have an opinion yet, anyway, that's to be discussed in PR #433

There is by the way at least one more bug, when the input is a reference.

There are more issues with this method, yes. I have a refactored version which should clear away the issue as well.

PR #434 would also fix this, as well as working for references.

@carlosmn ouch, just pushed my version, feel free to improve it

@Inkane please check if it works for you

@jdavid : Your change does indeed fix my issue.