On 02/09/2025 14:22, Phillip Wood wrote:
On 01/09/2025 21:43, Colin Stagner wrote:
On 9/1/25 08:54, Phillip Wood wrote:
Colin Stagner <ask+git@xxxxxxxxxxx> writes:
The outer loop in git-subtree.sh:983 appears to iterate from the root
commit forwards… and not from the HEAD backwards.
git rev-list --topo-order --reverse --parents $rev $unrevs
# ^^^^^^^^^
Since the iteration is ancestor-first, I'm having difficulty seeing
why `should_ignore_subtree_split_commit()` would want to do an
ancestor traversal at all. It already sees the commits ancestor-first.
But there could be a reason that I don't know.
Ah, I was only looking at this patch, not how it was called. That begs
the question "what's the point of these checks if we've already visited
all the ancestors anyway". I think the answer is that it is pruning the
recursion that happens in check_parent() and checking the commits that
come from that rev-list command is pointless. The regression test
introduced with this function only looks at $extracount which comes from
the recursion. I haven't looked too closely but it would be nice if we
could move this check so it is only run when check_parents() is recursing.
Sorry that's not quite right. check_parents() recurses into
process_split_commit() rather than the loop that call
should_ignore_subtree_split_commit(). I think what this check does do is
prune some parents which stops check_parents() from recursing into other
subtrees so the check is in the right place.
Thanks
Phillip
