From: Junio C Hamano <gitster@pobox•com>
To: Jeff King <peff@peff•net>
Cc: git@vger•kernel.org, John Fultz <jfultz@wolfram•com>
Subject: Re: git filter-branch not removing commits when it should in 2.7.0
Date: Tue, 19 Jan 2016 13:35:25 -0800 [thread overview]
Message-ID: <xmqq7fj59rs2.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <xmqqbn8h9squ.fsf@gitster.mtv.corp.google.com> (Junio C. Hamano's message of "Tue, 19 Jan 2016 13:14:33 -0800")
Junio C Hamano <gitster@pobox•com> writes:
> John Fultz <jfultz@wolfram•com> writes:
>
>> This seems to be a 2.7.0 regression in filter-branch. The bug is reproducible on Mac/Windows (haven't tried Linux) in the 2.7.0 production releases.
>>
>> Make an empty repo and put an empty commit in the history. E.g.,
>>
>> echo > foo && git add . && git commit -m "commit 1" && git commit --allow-empty -m "commit 2"
>>
>> Now try to use filter-branch to remove the empty commit. Both of the following methods leave master unchanged, but both worked in 2.6.4:
>>
>> git filter-branch --prune-empty
>> git filter-branch --commit-filter 'git_commit_non_empty_tree "$@"'
>
> Thanks.
>
> Since there were only 5 changes to git-filter-branch.sh between
> v2.6.0 and v2.7.0, it was fairly easy to pinpoint.
>
> Reverting the following commit from v2.7.0 seems to give the same
> result as v2.6.0 for "--prune-empty" experiment.
>
> commit 348d4f2fc5d3c4f7ba47079b96676b4e2dd831fc
> Author: Jeff King <peff@peff•net>
> Date: Fri Nov 6 01:24:29 2015 -0500
>
> filter-branch: skip index read/write when possible
>
> If the user specifies an index filter but not a tree filter,
> filter-branch cleverly avoids checking out the tree
> entirely. But we don't do the next level of optimization: if
> you have no index or tree filter, we do not need to read the
> index at all.
>
> This can greatly speed up cases where we are only changing
> the commit objects (e.g., cementing a graft into place).
> Here are numbers from the newly-added perf test:
>
> Test HEAD^ HEAD
> ---------------------------------------------------------------
> 7000.2: noop filter 13.81(4.95+0.83) 5.43(0.42+0.43) -60.7%
>
> Signed-off-by: Jeff King <peff@peff•net>
> Signed-off-by: Junio C Hamano <gitster@pobox•com>
OK, is this because git_commit_non_empty_tree() does this to decide
that it should skip the commit:
git_commit_non_empty_tree()
{
if test $# = 3 && test "$1" = $(git rev-parse "$3^{tree}");
then
map "$3"
else
git commit-tree "$@"
fi
}
where its parameters when --prune-empty is in use (or when the
function is used as the commit-filter), $1 is "$tree", $2 and $3 are
"-p" and its sole commit object name $commit (which is read from the
revs file, which is an output from rev-list, so it is known to be
40-hex) and tree after the said patch is computed like so:
if test -n "$need_index"
then
tree=$(git write-tree)
else
tree="$commit^{tree}"
fi
i.e. the helper does textual comparison between "$FOURTY_HEX^{tree}"
and 40-hex from rev-parse "$3^{tree}"?
In other words, would the fix be a one-liner like this?
git-filter-branch.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 98f1779..86b2ff1 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -404,7 +404,7 @@ while read commit parents; do
then
tree=$(git write-tree)
else
- tree="$commit^{tree}"
+ tree=$(git rev-parse "$commit^{tree}")
fi
workdir=$workdir @SHELL_PATH@ -c "$filter_commit" "git commit-tree" \
"$tree" $parentstr < ../message > ../map/$commit ||
next prev parent reply other threads:[~2016-01-19 21:35 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 [this message]
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
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=xmqq7fj59rs2.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=jfultz@wolfram$(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