From: Junio C Hamano <gitster@pobox•com>
To: Jeff King <peff@peff•net>
Cc: Jonathan Nieder <jrnieder@gmail•com>,
git@vger•kernel.org, John Fultz <jfultz@wolfram•com>
Subject: Re: [PATCH] filter-branch: resolve $commit^{tree} in no-index case
Date: Tue, 19 Jan 2016 19:23:35 -0800 [thread overview]
Message-ID: <xmqqziw153yg.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <xmqq4me96kd2.fsf@gitster.mtv.corp.google.com> (Junio C. Hamano's message of "Tue, 19 Jan 2016 18:43:53 -0800")
Junio C Hamano <gitster@pobox•com> writes:
> 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.
next prev parent reply other threads:[~2016-01-20 3:23 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-19 20:48 git filter-branch not removing commits when it should in 2.7.0 John Fultz
2016-01-19 21:14 ` Junio C Hamano
2016-01-19 21:35 ` Junio C Hamano
2016-01-19 21:37 ` Jeff King
2016-01-19 21:46 ` Junio C Hamano
2016-01-19 21:51 ` [PATCH] filter-branch: resolve $commit^{tree} in no-index case Jeff King
2016-01-19 21:59 ` Jeff King
2016-01-19 22:07 ` Jeff King
2016-01-19 22:23 ` Junio C Hamano
2016-01-19 22:28 ` Jeff King
2016-01-19 22:48 ` Jeff King
2016-01-20 1:22 ` Jonathan Nieder
2016-01-20 1:34 ` Jeff King
2016-01-20 1:51 ` Junio C Hamano
2016-01-20 2:00 ` Jeff King
2016-01-20 2:43 ` Junio C Hamano
2016-01-20 3:23 ` Junio C Hamano [this message]
2016-01-20 4:14 ` Jeff King
2016-01-20 0:47 ` Jonathan Nieder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqziw153yg.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=jfultz@wolfram$(echo .)com \
--cc=jrnieder@gmail$(echo .)com \
--cc=peff@peff$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox