Re: [PATCH] restore: invalidate cache-tree when removing entries with --staged

From: Dennis Kaarsemaker <dennis@kaarsemaker.net>
Date: Feb 5, 2020, 20:24 UTC
Message-ID: <8575e5067f7506992cc61ceb3b4185ab9c6f3d57.camel@kaarsemaker.net>
In-Reply-To: <20200108114344.GA3380580@coredump.intra.peff.net>

On Wed, 2020-01-08 at 06:43 -0500, Jeff King wrote:

Show 17 quoted lines

> On Wed, Jan 08, 2020 at 05:40:08AM -0500, Jeff King wrote:
> 
> > So there seem to be at least two bugs:
> > 
> >  - git-restore doesn't properly invalidate the cache-tree
> > 
> >  - the index-reading code is not careful enough about bogus cache-trees,
> >    and may segfault
> 
> Here's a fix for the first one. I'm adding Junio to the cc as an expert
> in index and cache-tree issues. I'm pretty sure this is the correct fix,
> but I have some lingering questions below.
> 
> I'm not planning on working on the second one immediately. Between this
> and Emily's patch from yesterday, I have a feeling that the index code
> could use an audit to be a bit more careful about handling bogus on-disk
> data.

We just ran into something similar where git would create really bogus commits when mixing squash merges and restore. As it's a private repo, I don't have an exact recipe for reproducing it, but it roughly goes like:

git checkout master git merge --squash branch-for-which-we-want-to-redo-commits git restore --staged . git add file1 file2 file3 git commit -m "commit"

This commit would remove a file4 that wasn't even touched in the branch (further commits would do even more broken things, eventually leading to broken commit objects with duplicate file contents). Changing `git restore --staged .` to `git reset HEAD` made this behaviour go away. A quick search in the list brought this patch and I'm happy to say it fixes our issue as well.

Thanks Peff!

D.