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

[Junio C Hamano <gitster@pobox.com>]

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.