Re: [PATCH] filter-branch: resolve $commit^{tree} in no-index case

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 20, 2016, 03:23 UTC
Message-ID: <xmqqziw153yg.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqq4me96kd2.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 5 quoted lines

> In other words, I do not think "broken tree object may look the same
> to diff-tree, but I do want to replace it" is relevant to this
> codepath; it is not something this function handles, I think.
>
> What am I not seeing?

One thing that I was not seeing is that "diff-tree --quiet" (without any magic options like -ignore-whitespace that may make two different tree object judged to be the same, and without -C -C that forces it to recurse into identical subtrees) lacks an obvious and stupid optimization to say "they are the same" or "they are different when given two tree object names without opening the tree(s). So if you make it compare a rewritten and corrected tree and an incorrect and possibly broken one, it may try to read the tree data from the latter and comparison can lead to incorrect results.

A consequence of this is if you are running filter-branch without any tree filters (i.e. no_index) but with "--prune-empty", and a commit and its parent actually have different trees but one (or both) of them are broken, "diff-tree" _might_ say "they are the same" and you end up skipping a commit when you do not want to. If your plan was to run another round of filter-branch, this time with a "broken tree encoding correction" tree-filter, on the result of the first "--prune-empty" filtering, we would definitely end up with a wrong history.

But for such a case, I would say you should be running the correction filter as the very first thing. So I am not sure if it matters in practice.

One possible action item out of this is that we might want to think about giving the obvious and stupid optimization to such invocation of "diff-tree --quiet". I _think_ we correctly avoid descending into the identical subtrees while doing a recursive diff-tree by saying "hey these two corresponding directories have the same tree object names", but there is no fundamental reason why we shouldn't be doing the same optimization at the top-level of the comparison.