On Sat, Oct 14, 2023 at 11:02:38PM +0200, Kristoffer Haugsbakk wrote:

Die gracefully when `git grep --no-index` is run outside of a Git
repository and the path is outside the directory tree.

If you are not in a Git repository and say:

    git grep --no-index search ..

You trigger a `BUG`:

    BUG: environment.c:213: git environment hasn't been setup
    Aborted (core dumped)

Because `..` is a valid path which is treated as a pathspec. Then
`pathspec` figures out that it is not in the current directory tree. The
`BUG` is triggered when `pathspec` tries to advice the user about the path
to the (non-existing) repository.

Is it even reasonable for "grep --no-index" to care about leaving the
tree in the first place? That is, is there a reason we should not allow:

  git grep --no-index foo ../bar

? And if we do want to care, there is a weirdness here that even with
your patch, we check to see if the file exists:

  $ git grep --no-index foo ../does-exist
  fatal: '../does-exist' is outside the directory tree

  $ git grep --no-index foo ../does-not-exist
  fatal: ../does-not-exist: no such path in the working tree.

If we want to avoid leaving the current directory, then I think we need
to be checking much sooner (but again, I would argue that it is not
worth caring about in no-index mode).

I do think your patch does not make anything worse (and indeed makes the
error output much better). So I do not mind it in the meantime. But I
have a feeling that we'd end up reverting it as part of the fix for the
larger issue.

-Peff