Skip to content

revwalk: fix uninteresting revs sometimes not limiting graphwalk#4622

Merged
ethomson merged 1 commit into
libgit2:masterfrom
pks-t:pks/revwalk-hide-newer-parents
Apr 16, 2018
Merged

revwalk: fix uninteresting revs sometimes not limiting graphwalk#4622
ethomson merged 1 commit into
libgit2:masterfrom
pks-t:pks/revwalk-hide-newer-parents

Conversation

@pks-t

@pks-t pks-t commented Apr 12, 2018

Copy link
Copy Markdown
Member

When we want to limit our graphwalk, we use the heuristic of checking
whether the newest limiting (uninteresting) revision is newer than the
oldest interesting revision. We do so by inspecting whether the first
item's commit time of the user-supplied list of revisions is newer than
the last added interesting revision. This is wrong though, as the user
supplied list is in no way guaranteed to be sorted by increasing commit
dates. This could lead us to abort the revwalk early before applying all
relevant limiting revisions, outputting revisions which should in fact
have been hidden.

Fix the heuristic by instead checking whether any of the limiting
commits was made earlier than the last interesting commit. Add a test.

When we want to limit our graphwalk, we use the heuristic of checking
whether the newest limiting (uninteresting) revision is newer than the
oldest interesting revision. We do so by inspecting whether the first
item's commit time of the user-supplied list of revisions is newer than
the last added interesting revision. This is wrong though, as the user
supplied list is in no way guaranteed to be sorted by increasing commit
dates. This could lead us to abort the revwalk early before applying all
relevant limiting revisions, outputting revisions which should in fact
have been hidden.

Fix the heuristic by instead checking whether _any_ of the limiting
commits was made earlier than the last interesting commit. Add a test.
@pks-t

pks-t commented Apr 12, 2018

Copy link
Copy Markdown
Member Author

This should fix #4333

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants