Re: [PATCH] revision.c: set-up "index_state.repo", don't segfault in pack-objects

[Emily Noneman <emily.noneman@gmail.com>]

The latest changes seem to have resolved the issue for me. My repo was
a bit different than Pauls, it's a big corporate repo with a partial
checkout and a worktree. What's confusing is that I do see an "index"
file in worktrees/NetLedger_GitRepo , but git 2.37.1 exhibits the
crash. 2.37.1.377.g679aad9e82 works just fine.

On Fri, Aug 5, 2022 at 12:42 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jeff King <peff@peff.net> writes:
>
> > Of the two patches, I think 4447d4129d is the better approach. The
> > assumption in the code seems to be that do_read_index() (and thus
> > read_index_from(), etc) will set up istate->repo. That patch fixes a
> > corner case where we failed to do so. And with that fix, there's no need
> > for the callers to set things up ahead of time. So it covers all of
> > those initializers you mentioned.
>
> Yeah, I tend to agree that Martin's fix, which is a more focused
> one, is the better approach between the two.  It was merged to
> the 'master' track only a few days ago.
>
> We are at the end of the week #4 of this 12-week cycle, and we've
> accumulated about two dozens of topics already on 'maint' since the
> first maintenance release v2.37.1 was done, so we may have a chance
> to merge Martin's fix down to 'maint' before tagging the v2.37.2
> release.
>
> One thing that may help is to add a test similar to the situation
> Emily & Paul had to t7063 on top of 4447d4129d before merging it
> down, perhaps?
>
> In your reproduction, the "rm .git/index" step makes the worktree's
> branch "not checked out" (the commit is empty so "worktree add" may
> check out no files, and removing the index will make it as if you
> did "clone --no-checkout") ...
>
>   git init repo
>   cd repo
>   git commit --allow-empty -m base
>
>   git config core.untrackedCache true
>   git worktree add at
>   rm .git/worktrees/wt/index
>
>   git gc
>
> ...  but it is not something an end user is likely to do, so I am
> still curious how this was triggered in real life.
>
> Ah, OK, I think the steps can be tweaked to
>
>       git config core.untrackedCache true
>     - git worktree add at
>     - rm .git/worktrees/wt/index
>     + git worktree add --no-checkout wt
>
>       git gc
>
> i.e. (1) With "worktree add --no-checkout", there is no need to
> manually remove the index file, and (2) "at" is an obvious typo of
> "wt".  This does not require the history to be a singleton empty
> tree, either.
>
> And that is a less implausible thing for users to be doing, but may
> still not be very common.  Perhaps the --no-checkout is a prelude to
> set up a custom sparse checkout pattern, to avoid a wasteful full
> tree checkout before doing so, or something?  If that is the case,
> then the above sequence becomes a very plausible thing for users to
> be doing.
>
> > Emily, Paul: I'm 99% sure this will be the case given my reproduction
> > above, but if you could try reproducing the problem with the current tip
> > of "master" from git.git, that would confirm the findings.
>
> Yes, indded.  That would be very helpful.
>
> Thanks, all, for discussing the problem and its solution(s), and
> thanks in advance for further testing to help the fix forward.