Fix missing parents #19
Conversation
Classes/git/PBGitRevList.mm
Outdated
| GTCommit *branchCommit1 = obj1; | ||
| GTCommit *branchCommit2 = obj2; | ||
|
|
||
| return [branchCommit1.commitDate compare:branchCommit2.commitDate]; |
There was a problem hiding this comment.
I have to admit I do not understand the surrounding code and what sort it tries to achieve … but this seems to revert the sort order.
WIth the preceding commits and reverts changing this behaviour, I am a bit confused, so perhaps an explanation would be good.
(As it’s unclear to me from which direction the resulting list will be traversed, I cannot figure out whether newest first or oldest first would be the correct way to go here.)
|
Just so I'm clear, this is a tentative fix. I'm currently trying to pinpoint a reproduction testcase so I can concentrate on looking at bad graphs instead of hoping that the issue show up. At the moment, I think #17 happens when you have a branch (B) based on another branch (A), and you rebase (maybe with fixups?) all of them against your current master. You'll have commits from B that are older than the current commit from A and the graph code will be confused. But I've seen another really weird issue while running that one too, so I'm not sure anymore if the fix is correct. |
|
@tiennou : By all means, give it a shot! I am seeing this issue regularly (working around it by reloading). In case that helps for your theories: my typical workflow includes doing git pull --rebase and I tend to see the issue after doing that.
Let me know if doing any particular tests would be helpful. |
|
If you can make it go away by reloading, I think it's not this issue (it's another one I'm aware of though 😉). The underlying problem is that :
For the record, the initial switch |
tiennou
force-pushed
the
fix/missing-parents
branch
from
236db8b to
d6d5d06
Compare
|
I think I got it. It was waaaay easier than I thought 😆. I'll be running that along with a bunch of other branch, so I'll see if it really went away. @ssp Thinking about it, this is what I saw just before ASan triggered (the report I posted came from that). I'm not sure what happens though (apart from ASan being unhappy), I'd suspect we have a bunch of objects pointing to old OIDs that get graphed at the same time as the new ones, but a refresh removes the old ones. Do you have the "auto-refresh" thingy enabled ? |
|
@tiennou looking forward to it! I seem to have run into this issue more often recently. |
While investigating gitx#17, I found that code pretty hard to understand. For the record, it built a set of OIDs while extracting the meaning of the `PBGitRevSpecifier`, which (since sets have no order) I suspected could cause the out-of-order behavior we had. Turns out the fix was much easier, but since I find this one cleaner, and less "loopy", here it is.
tiennou
force-pushed
the
fix/missing-parents
branch
from
d6d5d06 to
4bc3720
Compare
This reverts a change I did recently w.r.t to how we enumerate commits.
This should make the "broken" lane bug go away, at the expense of more lanes in some cases. As for the underlying issue, I don't really know why this happens. I suspect libgit2 doesn't revwalk in the same way as Git does, but I don't really have time to check what really differs...