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

From: Jeff King <peff@peff.net>
Date: Jan 20, 2016, 04:14 UTC
Message-ID: <20160120041447.GA12811@sigill.intra.peff.net>
In-Reply-To: <xmqqziw153yg.fsf@gitster.mtv.corp.google.com>

On Tue, Jan 19, 2016 at 07:23:35PM -0800, Junio C Hamano wrote:

Show 9 quoted lines

> 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.

Yeah, that is the case I was thinking about.

> 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.

I'd also agree with that.

Show 7 quoted lines

> 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.

Yeah, I just assumed we did, but a simple strace shows that is not the case. I doubt it matters _too_ much in practice, but it would probably be easy to do.

-Peff