Re: [PATCH 7/7] Implement git commit as a builtin command.

From: Kristian Høgsberg <krh@redhat.com>
Date: Sep 24, 2007, 20:27 UTC
Message-ID: <1190665647.11372.3.camel@hinata.boston.redhat.com>
In-Reply-To: <7vwsujizli.fsf@gitster.siamese.dyndns.org>

On Fri, 2007-09-21 at 12:32 -0700, Junio C Hamano wrote:

Show 28 quoted lines

> Kristian Høgsberg <krh@redhat.com> writes:
> 
> >> > +
> >> > +	/* update the user index file */
> >> > +	add_files_to_cache(fd, files, prefix);
> >> > +
> >> > +	if (!initial_commit) {
> >> > +		tree = parse_tree_indirect(head_sha1);
> >> > +		if (!tree)
> >> > +			die("failed to unpack HEAD tree object");
> >> > +		if (read_tree(tree, 0, NULL))
> >> > +			die("failed to read HEAD tree object");
> >> > +	}
> >> 
> >> Huh?  Doesn't this read_tree() defeat the add_files_to_cache()
> >> you did earlier?
> >
> > This is the case where we add the files on the command line
> > to .git/index, but commit from a clean index file corresponding to HEAD
> > with the files from the command line added (partial commit?).  The first
> > add_files_to_cache() updates .git/index, then we do read_tree() to build
> > a tmp index from HEAD and then we add the files again.  The tmp index is
> > written to a tmp index file.
> 
> Still, if you are doing read_tree() that reads into the same
> in-core cache you have just prepared in the add_fiels_to_cache()
> above, potentially overwriting whatever you did, doesn't it?
> That was what I was puzzled about...

Ah, I understand the confusion - add_files_to_cache() will write out the cache to the given fd and close it. That's not clear, and I've moved the write+close part back into prepare_index() in the follow-on patches I sent that shares out add_files_to_cache() with builtin-add.c.

Show 7 quoted lines

> > ...  As for just using an in-memory
> > index, I wanted to do it that way originally, but you have to write it
> > to disk after all for the pre-commit hook.
> 
> Ah, I completely forgot about the hook.  Ok, scratch the idea of
> not using a temporary index file.  The is not much potential for
> performance gain anyway.

Ok, cool, I'll keep the current structure of the code then.

Kristian