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
