Repository.Commits.QueryBy is missing changes

[hybridherbst]

Reproduction steps

This seems to have been brought up a couple of times already (a quick search showed #1401, #1591, https://github.com/libgit2/libgit2sharp/pull/963#issuecomment-355674649).

This is pretty weird: QueryBy returns 0 LogEntries, while going through all commits and diffing to the previous one shows the same changes/additions/renames as e.g. the SourceTree or Fork UI for the exact same path as for the QueryBy call.

Note that it only does that for some files, most files work fine. I haven't found a common pattern so far (e.g. specific characters, or only merges, or ...) but it looks like QueryBy is omitting merges and that might cause problems.

(commits in reverse order)

Expected behavior

QueryBy should return all changes to the file (as outlined in #963 where it was originally added).

Actual behavior

QueryBy returns 0 LogEntries.

Version of LibGit2Sharp (release number or SHA1)

git2-106a5f2

Operating system(s) tested; .NET runtime tested

Win 10, .NET 4.6

EDIT: Maybe I'm understanding the code wrong, but taking a quick peek at https://github.com/libgit2/libgit2sharp/blob/master/LibGit2Sharp/Core/FileHistory.cs#L137, I think QueryBy in fact ignores all commits that have more than 1 parent (e.g. all merges).

[hybridherbst]

After looking into the code and fixing it for my usecase (showing ALL commits touching the file, as I had expected), I think there are multiple issues and ideally the behaviour should be configurable to avoid confusion:

• QueryBy ignores commits with multiple parents (e.g. merges) and never walks into both parents
• QueryBy stops when the current tree does not contain the file

This leads to the following case failing:

• add A on "master"
• do some changes on A
• branch off a branch "develop"
• change A on "develop"
• go back to "master"
• remove A on "master"
• merge "develop" into "master", choose "theirs" on conflict resolution

Now "master" contains A again, which has been changed quite a lot. But the current QueryBy implementation hides all those changes, returning 0 commits, since it does not see the merge and stops searching through master where the file was not in the tree (when it was deleted).

I think ignoring the merge is a bug. As far as I'm aware merges can contain anything - changes to the file, file additions, deletions ... it's pretty easy to hide a lot of changes this way: (QueryBy still returns 0 changes on TestScript2)

In some cases QueryBy will return some commits but falsely ignore most of them: (QueryBy returns only 1 commit - "reset TestScript2 to earlier state". All commits seen here do something with TestScript2.)

From a performance point of view, I understand that stopping when the file is not in the tree improves performance a lot on large repos. I think the code needs to become a bit smarter here and only stop if the file is not in the tree and there has been no commit seen yet which "added" the file.

[ErikSchierboom]

I've recently also hit this issue, and it would be great if there would be a fix.

[lofcz]

tested, with this PR merges were - at least in my cases - correctly reffed. For a quick test you can swap nuget to: https://www.nuget.org/packages/LibGit2SharpLofcz/0.27.0-preview-0113 (built against this PR and net6)