git.log: a commit past the parent bound fails the call even when `since` excludes it
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 9
- Forks
- 0
- Avg merge
- 3h 3m
- Merged PRs (30d)
- 509
Description
Raised by Codex on #681, against the parent bound that PR moved into the traversal. The move is right — checked any later, the parents are already on the walk's stack, which is the allocation the bound exists to refuse — but it lands the check ahead of a filter that could have made it unnecessary.
The interaction
walkPage layers iterators: multiRootCommitIter traverses, newPathFilteringCommitIter may wrap it, and object.NewCommitLimitIterFromIter applies since on the outside. since is therefore evaluated after the traversal has already popped a commit and expanded its parents.
So for a repository with an octopus merge of more than maxLogParents (64) parents, a git.log call fails with InvalidInput even when the caller's since window ends well after that commit — and the remedy the diagnostic offers ("narrow the walk with since or path") is exactly the thing that cannot help, because the cutoff never gets to speak. The recent, in-range commits the caller asked for are not returned at all.
Rare, but not hypothetical: octopus merges past 64 parents exist in real history.
Why it was not fixed on the PR
The fix is to give the traversal the cutoff, so an out-of-range commit stops the walk before the parent expansion is reached — which means multiRootCommitIter growing a notion of since (and, for the same reason, of the path filter) rather than those staying layers above it. That is a change to how the three iterators compose, not a line inside the bound, and #681 is a bounds PR.
The current behaviour is at least the fail-closed direction: it refuses rather than silently omitting commits, and #681's other half makes sure it refuses as an error rather than as a cursor that can never make progress.
Shape of a fix
Push the since cutoff (and ideally the path predicate) into multiRootCommitIter so the traversal can decline to expand a commit it is not going to emit anyway, leaving the parent bound to fire only for commits genuinely in range.
Codex thread: https://github.com/picatz/flowstate/pull/681#discussion_r3790548627
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading multiRootCommitIter and how it is layered with newPathFilteringCommitIter and object.NewCommitLimitIterFromIter in walkPage. Reproduce an octopus merge with more than 64 parents using a git.log call with since, then verify that out-of-range commits do not trigger the parent bound while in-range commits still do.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- git, go
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100