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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Johannes Sixt <j.sixt@xxxxxxxxxxxxx> writes:

> From: Johannes Sixt <j6t@xxxxxxxx>
>
> 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@xxxxxxxx>

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).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]