Re: [PATCH 1/2] filter-branch: remove an unnecessary use of 'git read-tree'

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 16, 2009, 00:19 UTC
Message-ID: <7v1vivydlg.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4B274BDE.8000504@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

Show 10 quoted lines

> From: Johannes Sixt <j6t@kdbg.org>
>
> The intent of this particular call to 'git read-tree' was to fill an
> index. But in fact, it only allocated an empty index. Later in the
> program, the index is filled anyway by calling read-tree with specific
> commits, and considering that elsewhere the index is even removed (i.e.,
> it is not relied upon that the index file exists), this first call of
> read-tree is completely redundant.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>

Very true. The only thing it would have done is to error out early when the user mistakenly tried to run the command in a directory in which s/he does not have write access to, before running potentially expensive commit listing with rev-list, but then the user would have failed to create the tempdir before that to begin with.

Will queue, but it doesn't seem urgent to put it in 1.6.6 or 1.6.5.X maint (i.e. nothing is broken without this patch).

>  Calling read-tree without arguments is not allowed according to the
>  documentation.

I think saying "is not allowed" is going a bit too far. The documentation simply does not list it as a _useful_ thing to do, that's all.

By the way, I notice that the command still insists on being run in a clean work tree with a clean index at the beginning, but isn't everything done inside the tempdir (i.e. ".git-rewrite" by default) these days, including the temporary work tree tree-filter creates with "checkout-index -fqua"? It is obviously not a topic of this patch, but we may want to stop doing that if we are not rewriting the current commit (which we will know by the time we list the commits to be rewritten with rev-list before actually starting to rewrite).