Re: [PATCH 2/4] reset: use "unpack_trees()" directly instead of "git read-tree"

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

 



On Fri, 11 Sep 2009, Christian Couder wrote:

> On Friday 11 September 2009, Junio C Hamano wrote:
> > Christian Couder <chriscool@xxxxxxxxxxxxx> writes:
> > > From: Stephan Beyer <s-beyer@xxxxxxx>
> > >
> > > This patch makes "reset_index_file()" call "unpack_trees()" directly
> > > instead of forking and execing "git read-tree".
> >
> > And the reason why it is a good idea is...?
> 
> ...that it should be faster.
> 
> Ok, I will add that to the commit message in the next version.

There's also the benefit (IMHO, more significant) that git-read-tree's 
command line parsing is complicated, and using it from git-reset makes it 
hard to see exactly what each option of "git reset" does in terms of 
operations on the index and the working tree.

For example, it's not obvious when reading the code to run read-tree that 
all of the branches lead to the use of the "merge" flag, because some 
branches use "-m" and some use "--reset".

Actually, there's a behavior difference between the old version and the 
new version. The old version gives an error for "git reset --merge" with 
unmerged entries (unlike any other option to "git reset", AFAICT), and the 
new version does not. There's no way you'd know this without a careful 
reading of cmd_read_tree() and cross-reference with reset_index_file(), 
since the documentation doesn't mention it, the code in reset_index_file() 
only replaces "--reset" with "-m", and it seems to be doing that for the 
effect of not ignoring differences in the working tree.

	-Daniel
*This .sig left intentionally blank*
--
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]