Re: [PATCH] Add read_cache to builtin-check-attr

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Aug 14, 2007, 14:46 UTC
Message-ID: <Pine.LNX.4.64.0708141540420.25989@racer.site>
In-Reply-To: <20070814142428.GK21692@lavos.net>

Hi,

On Tue, 14 Aug 2007, Brian Downing wrote:

Show 11 quoted lines

> On Tue, Aug 14, 2007 at 03:08:52PM +0100, Johannes Schindelin wrote:
> > Shouldn't read_cache() be _only_ called if
> > 
> > - it has not been read yet, and
> > - .gitattributes was not found in the work tree?
> > 
> > IOW check-attr is the wrong place for your patch IMHO.
> 
> I admit I just cargo-culted what builtin-checkout-index did upon starting.
> Off the cuff, though, I don't see how the cache could ever already be
> loaded upon the start of cmd_check_attr,

Right. I was talking more about read_cache() being called later anyway, so you do not have to read the cache if a .gitattributes is there and you do not need the index to begin with.

> and the way the attr.c code is
> written, the cache be loaded when we check attributes or it will default
> to the old behavior (only checking the working directory.)

Why not just make sure that the index is read in read_index_data()? Something like

	/* read index if that was not already done yet */
	if (!istate->mmap)
		read_index(&istate);

(Yes, I know that read_index() calls read_index_from(), which in turn checks that, but read_attr() is called possibly pretty often, right? So we might just as well spare a few cycles here.)

Ciao, Dscho