Re: [RFC PATCH] Not computing changed path filter for root commits
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Oct 10, 2023, 19:47 UTC
- Message-ID
- <ZSWqStEpsFU0SmEm@nand.local>
- In-Reply-To
- <20231009205925.1915096-1-jonathantanmy@google.com>
On Mon, Oct 09, 2023 at 01:59:25PM -0700, Jonathan Tan wrote:
Show 27 quoted lines
> Taylor Blau <me@ttaylorr.com> writes: > > This only happens when we return REV_TREE_NEW from a call to > > `rev_compare_tree(revs, p, commit, nth_parent)`. But we'll only get > > REV_TREE_NEW back if > > > > repo_get_commit_tree(the_repository, p); > > > > returns NULL. But when we call rev_same_tree_as_empty(revs, p) in the > > REV_TREE_NEW case, we return early as follows: > > > > struct tree *t1 = repo_get_commit_tree(revs, p); > > if (!t1) > > return 0; > > > > So we won't even consult the Bloom filter in that case, since t1 is NULL > > for the same reason as what caused rev_compare_tree() to return > > REV_TREE_NEW in the first place. > > > > I am still dumbfounded by how we would ever get REV_TREE_NEW in the > > first place, but if we did, I think we would be OK here. > > > > Thanks, > > Taylor > > Ah, good point. Your patch in > https://lore.kernel.org/git/ZQnmTXUO94%2FQy8mq@nand.local/ looks good to > me, then.
Oops, I made a mistake in the quoted portion, which is that we could get REV_TREE_NEW if the tree-diff itself only adds files. This is the non-trivial case that we get when t1 is non-NULL, and we end up calling `diff_tree_oid()` which sets the static `tree_difference` variable.
So I think adding an nth_parent field (like you originally suggested[^1]) makes sense.
Thanks, Taylor
[^1]: Thanks for being patient with me ;-).