Re: [PATCH v2] commit: skip discarding the index if there is no pre-commit hook

From: Jeff King <peff@peff.net>
Date: Aug 15, 2017, 04:53 UTC
Message-ID: <20170815045313.njpq5wf7iuredhoe@sigill.intra.peff.net>
In-Reply-To: <DM2PR21MB004160EA994A445A89BD50F7B78D0@DM2PR21MB0041.namprd21.prod.outlook.com>

On Tue, Aug 15, 2017 at 04:23:50AM +0000, Kevin Willford wrote:

Show 11 quoted lines

> > This read_cache_from() should be a noop, right, because it immediately
> > sees istate->initialized is set? So it shouldn't matter that it is not
> > in the conditional with discard_cache(). Though if its only purpose is
> > to re-read the just-discarded contents, perhaps it makes sense to put it
> > there for readability.
> 
> I thought about that and didn't know if there were cases when this would be called
> and the cache has not been loaded.  It didn't look like it since it is only called from 
> cmd_commit and prepare_index is called before it.  Also if in the future this call would
> be made when it had not read the index yet so thought it was safest just to leave
> this as always being called since it is basically a noop if the istate->initialized is set.

Yeah, I agree it might be safer as you have it. And hopefully the comment above the discard would point anybody in the right direction.

> Also based on this commit
> https://github.com/git/git/commit/2888605c649ccd423232161186d72c0e6c458a48
> it looked like the discard_cache was added independent of the read_cache_from call,
> which made me think that the two were not tied together.

Yeah, I tried to dig in the history and figure it out, but it was not nearly as clear as I would have liked. :)

-Peff